[MWAR-443] Fix deletion of files placed by maven-dependency-plugin - #642
Open
elharo wants to merge 2 commits into
Open
[MWAR-443] Fix deletion of files placed by maven-dependency-plugin#642elharo wants to merge 2 commits into
elharo wants to merge 2 commits into
Conversation
Fixes apache#522 Address code review comments: - Extract shared isLibraryType() in AbstractWarPackagingTask to avoid duplicating artifact type list across the codebase - Use outputFileNameMapping in getRuntimeArtifactFileNames() when set - Use isLibraryType() in both ArtifactsPackagingTask and the outdated resource detection logic - Fix test cleanup with try/finally and recursive delete
elharo
commented
Jul 28, 2026
| targetFileName = targetFileName.substring(0, targetFileName.lastIndexOf('.')) + ".jar"; | ||
| } | ||
| fileNames.add(targetFileName); | ||
| } catch (Exception e) { |
Contributor
Author
There was a problem hiding this comment.
shouldn't need to catch a raw java.lang.Exception here. be more specific about the exception type
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Fixes MWAR-443 by preventing the WAR plugin’s outdated-resource cleanup from deleting dependency JARs in WEB-INF/lib/ that were placed there by maven-dependency-plugin (not by the WAR plugin) when those files predate the session start time.
Changes:
- Added runtime-artifact filename computation and used it to avoid marking non-runtime lib files as outdated during the cleanup scan.
- Centralized “library type” detection into a shared
isLibraryType()helper and refactored packaging logic to use it. - Added a regression test covering the provided-scope dependency-plugin scenario.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/main/java/org/apache/maven/plugins/war/AbstractWarMojo.java | Avoids deleting WEB-INF/lib files unless they match expected runtime artifact filenames; adds runtime filename computation. |
| src/main/java/org/apache/maven/plugins/war/packaging/AbstractWarPackagingTask.java | Extracts shared isLibraryType() helper for library artifact classification. |
| src/main/java/org/apache/maven/plugins/war/packaging/ArtifactsPackagingTask.java | Refactors artifact copy routing to use isLibraryType() and keeps special handling for par. |
| src/test/java/org/apache/maven/plugins/war/WarExplodedMojoTest.java | Adds MWAR-443 regression coverage to ensure provided-scope JARs aren’t deleted. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
101
to
105
| } else if (isLibraryType(type)) { | ||
| if ("par".equals(type)) { | ||
| targetFileName = targetFileName.substring(0, targetFileName.lastIndexOf('.')) + ".jar"; | ||
| } | ||
| copyFile(id, context, artifact.getFile(), LIB_PATH + targetFileName); |
Comment on lines
+735
to
+738
| if ("par".equals(artifact.getType())) { | ||
| targetFileName = targetFileName.substring(0, targetFileName.lastIndexOf('.')) + ".jar"; | ||
| } | ||
| fileNames.add(targetFileName); |
Comment on lines
+684
to
+690
| if (normalizedPath.startsWith("WEB-INF/lib/") | ||
| && !runtimeArtifactFileNames.contains( | ||
| file.toFile().getName())) { | ||
| // Skip: file was placed by another plugin, not managed by WAR plugin | ||
| } else { | ||
| outdatedResources.add(path); | ||
| } |
Comment on lines
+982
to
+986
| } finally { | ||
| // Cleanup | ||
| providedJar.delete(); | ||
| libDir.delete(); | ||
| } |
- Invert the outdated resource detection to protect non-runtime-scope library artifacts instead of filtering out runtime-scope artifacts: stale files that are not part of the current project (e.g. outdated versions of runtime artifacts) are still removed. - Move the fileNameMapping evaluation and the par to jar conversion into shared helpers in AbstractWarPackagingTask and reuse them from both the outdated resource detection and ArtifactsPackagingTask. - Catch InterpolationException instead of Exception when evaluating the file name mapping. - Strengthen the tests: use a real project stub, cover the outputFileNameMapping case and the removal of stale runtime files, and clean up test resources recursively.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #522
Problem
When maven-dependency-plugin copies provided-scope JARs to
WEB-INF/lib/, the WAR plugin's outdated-resource cleanup deletes them because they were not copied by the WAR plugin itself and their timestamps predate the session start time.Solution
In
DefaultWarPackagingContext, skip adding files underWEB-INF/lib/tooutdatedResourcesunless they match an expected runtime-artifact filename. A newgetRuntimeArtifactFileNames()method computes the expected filenames from the set of runtime-scope library artifacts, respectingoutputFileNameMappingif configured.Changes
AbstractWarMojo.java: AddedgetRuntimeArtifactFileNames(); modifiedFileVisitorto skip marking non-artifact lib files as outdated.AbstractWarPackagingTask.java: ExtractedisLibraryType()for reuse.ArtifactsPackagingTask.java: Refactored to use sharedisLibraryType().WarExplodedMojoTest.java: Added regression test.